fix(accounts): validate payment entry references with latest data. #31166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dj12djdjs
requested review from
marination,
rohitwaghchaure,
ankush,
nextchamp-saqib and
deepeshgarg007
as code owners
|
I'm not sure if the tests are failing from this PR. I'll have a further look today. |
|
I really don't know why those two tests keep failing. They don't fail on my local machine. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #31166 +/- ##
==========================================
Coverage ? 64.11%
==========================================
Files ? 811
Lines ? 61870
Branches ? 0
==========================================
Hits ? 39669
Misses ? 22201
Partials ? 0
|
deepeshgarg007
requested changes
Jun 9, 2022
|
@deepeshgarg007 I've added another check incase the a reference get's partially allocated between two payment entries. I'm a confused why Output: Shouldn't the outstanding be the same as the sales invoice outstanding (100) and the allocated be 50 ? |
|
This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
|
This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
|
This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
|
@dj12djdjs @deepeshgarg007 any update on this? |
|
@rtdany10 I think my solution to this isn't getting accepted. I'll think about it and try submitting a different solution. |
anandbaburajan
force-pushed
the
fix-31138
branch
from
07999f6
to
86c1a80
Compare
anandbaburajan
added
backport version-13-hotfix
backport version-14-hotfix
deepeshgarg007
previously approved these changes
Jun 13, 2023
Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com>
|
Thanks @dj12djdjs! |
mergify bot
pushed a commit
that referenced
this pull request
Jun 13, 2023
…31166) * test: payment entry over allocation. * fix: validate allocated_amount against latest outstanding amount. * fix: payment entry get outstanding documents for advance payments * fix: only fetch latest outstanding_amount. * fix: throw if reference is allocated * test: throw error if a reference has been partially allocated after inital creation. * chore: test name * fix: remove unused part of test * chore: linter * chore: more user friendly error messages * fix: only validate outstanding amount if partly paid and don't filter by cost center * chore: minor refactor for doc.cost_center Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> --------- Co-authored-by: Anand Baburajan <anandbaburajan@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> (cherry picked from commit 20de27d)
mergify bot
pushed a commit
that referenced
this pull request
Jun 13, 2023
…31166) * test: payment entry over allocation. * fix: validate allocated_amount against latest outstanding amount. * fix: payment entry get outstanding documents for advance payments * fix: only fetch latest outstanding_amount. * fix: throw if reference is allocated * test: throw error if a reference has been partially allocated after inital creation. * chore: test name * fix: remove unused part of test * chore: linter * chore: more user friendly error messages * fix: only validate outstanding amount if partly paid and don't filter by cost center * chore: minor refactor for doc.cost_center Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> --------- Co-authored-by: Anand Baburajan <anandbaburajan@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> (cherry picked from commit 20de27d) # Conflicts: # erpnext/accounts/doctype/payment_entry/test_payment_entry.py
This was referenced Jun 13, 2023
deepeshgarg007
pushed a commit
that referenced
this pull request
Jun 13, 2023
…31166) fix(accounts): validate payment entry references with latest data. (#31166) * test: payment entry over allocation. * fix: validate allocated_amount against latest outstanding amount. * fix: payment entry get outstanding documents for advance payments * fix: only fetch latest outstanding_amount. * fix: throw if reference is allocated * test: throw error if a reference has been partially allocated after inital creation. * chore: test name * fix: remove unused part of test * chore: linter * chore: more user friendly error messages * fix: only validate outstanding amount if partly paid and don't filter by cost center * chore: minor refactor for doc.cost_center Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> --------- Co-authored-by: Anand Baburajan <anandbaburajan@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> (cherry picked from commit 20de27d) Co-authored-by: Devin Slauenwhite <devin.slauenwhite@gmail.com>
anandbaburajan
added a commit
that referenced
this pull request
Jun 13, 2023
…ackport #31166) (#35674) * fix(accounts): validate payment entry references with latest data. (#31166) * test: payment entry over allocation. * fix: validate allocated_amount against latest outstanding amount. * fix: payment entry get outstanding documents for advance payments * fix: only fetch latest outstanding_amount. * fix: throw if reference is allocated * test: throw error if a reference has been partially allocated after inital creation. * chore: test name * fix: remove unused part of test * chore: linter * chore: more user friendly error messages * fix: only validate outstanding amount if partly paid and don't filter by cost center * chore: minor refactor for doc.cost_center Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> --------- Co-authored-by: Anand Baburajan <anandbaburajan@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> (cherry picked from commit 20de27d) # Conflicts: # erpnext/accounts/doctype/payment_entry/test_payment_entry.py * chore: resolve conflicts * chore: resolve more conflicts * chore: don't validate allocated amount in case of donation --------- Co-authored-by: Devin Slauenwhite <devin.slauenwhite@gmail.com> Co-authored-by: Deepesh Garg <deepeshgarg6@gmail.com> Co-authored-by: Anand Baburajan <anandbaburajan@gmail.com>
frappe-pr-bot
pushed a commit
that referenced
this pull request
Jun 14, 2023
## [14.27.2](v14.27.1...v14.27.2) (2023-06-14) ### Bug Fixes * `enqueue_after_commit` wherever it makes sense (backport [#35588](#35588)) ([#35590](#35590)) ([e505516](e505516)) * `TypeError` in Closing Stock Balance ([32e5bbb](32e5bbb)) * **accounts:** validate payment entry references with latest data. ([#31166](#31166)) ([4add1b4](4add1b4)) * added process loss in job card ([6a21d61](6a21d61)) * allow user to set rounding loss allowance for accounts balance ([cf14858](cf14858)) * attribute error on payment reconciliation tool ([25b3c77](25b3c77)) * based on status_update.py update opportunity status to converted… ([#35145](#35145)) ([dee8275](dee8275)) * calculate wdv depr schedule properly for existing assets [v14] ([#35613](#35613)) ([feb5d00](feb5d00)) * conflicts ([2060a00](2060a00)) * CSS not applied to product title ([#35630](#35630)) ([2cf871c](2cf871c)) * don't set default payment amount in case of invoice return ([#35645](#35645)) ([79483cc](79483cc)) * Lower deduction certificate not getting applied ([#35667](#35667)) ([6f59fa9](6f59fa9)) * Make difference entry button not working ([#35622](#35622)) ([043815e](043815e)) * make showing taxes as table in print configurable (backport [#35672](#35672)) ([#35678](#35678)) ([f39ae9d](f39ae9d)) * Payment against credit notes will be considered as payment against parent invoice in Accounts Receivable/Payable report ([#35642](#35642)) ([81ef2ba](81ef2ba)) * Project in item-wise sales register ([#35596](#35596)) ([7737b90](7737b90)) * Stock Reconciliation document update while reposting ([8b617fb](8b617fb)) * test case ([7af0380](7af0380)) * Update de.csv ([#35278](#35278)) ([2077f6e](2077f6e)) * Validation for delivery date in Sales Order ([#35597](#35597)) ([4a8ce22](4a8ce22))
frappe-pr-bot
pushed a commit
that referenced
this pull request
Jun 14, 2023
## [13.51.2](v13.51.1...v13.51.2) (2023-06-14) ### Bug Fixes * **accounts:** validate payment entry references with latest data. (backport [#31166](#31166)) ([#35674](#35674)) ([4d4f218](4d4f218)) * Asset Depreciation Ledger Report - Add Total Row Checkbox Enabled ([3831c79](3831c79)) * calculate wdv depr schedule properly for existing assets [v13] ([#35615](#35615)) ([97f4af8](97f4af8)) * CSS not applied to product title (backport [#35582](#35582)) ([#35635](#35635)) ([1b69b37](1b69b37)) * don't set default payment amount in case of invoice return (backport [#35645](#35645)) ([#35648](#35648)) ([8e3636f](8e3636f)) * Lower deduction certificate not getting applied ([#35667](#35667)) ([c2bf8e3](c2bf8e3)) * make showing taxes as table in print configurable ([#35672](#35672)) ([4c2c037](4c2c037)) * Project in item-wise sales register ([#35596](#35596)) ([9d5b500](9d5b500)) * savepoint policy assignment submission, log errors & inform the user about failures ([#35507](#35507)) ([4a35ff0](4a35ff0)) ### Performance Improvements * refactor `get_all_nodes` in Org Chart ([986a90e](986a90e))
|
Payment Entry against Employee Advance (HR) stopped functioning after this PR: 
Fixed in: #35741 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #31138
Problem:
The system allows multiple draft payment entries to be made for a single invoice, which is fine, but when the first payment entry is submitted, the outstanding amount of the invoice in the second payment entry isn't accurate anymore. But the system still allows the user to submit the entry, resulting in negative outstanding amounts.
Solution:
During validation of allocated amount in the payment entry, update each row in references with latest information before comparing amounts. If the user tries to submit another payment entry but the corresponding invoice has already been fully paid, the system would throw an error to let the user know. If the invoice has been already partly paid, the user would be told the same and would be asked to use the 'Get Outstanding Invoice' button to get the latest outstanding amount.
Unrelated: I discovered fetching documents for advance payment via payment entry form was also broken. Fixed in 1ca20b9